Repository navigation
feat: set and shopts directives - #929
Conversation
andreynering
left a comment
There was a problem hiding this comment.
First look.
I liked the direction here, but adding interp.BashOpts to mvdan/sh first would be great IMO.
| // Manually adding bash opts before each command as there is no interp.Shopts | ||
| if len(opts.BashOpts) > 0 { | ||
| s = append(s, fmt.Sprintf("shopt -s %s", strings.Join(opts.BashOpts, " "))) | ||
| } |
There was a problem hiding this comment.
I agree on contributing to mvdan/sh adding a interp.BashOpts option. I think it may be even worth waiting for it to be released before merging this here (but if it takes too long, it's fine to merge as is and adapt later).
|
@andreynering I've fixed the issues you raised, but haven't had the time to look into the codebase over at mvdan/sh yet. I've opened an issue (mvdan/sh#962) over there instead. Hopefully I (or someone else) will have the time to work on this soon. In the meantime, if you're happy to merge this as-is, I can open another issue to track the improvements once (if) the functionality is added upstream. |
- Rename `shopts` to `shopt` - Move `UniqueJoin` to a more appropriate package - Write documentation - Add entry to README
andreynering
left a comment
There was a problem hiding this comment.
Yes, let's get this merged and we can improve once mvdan/sh#962 gets implemented.
I've renamed shopts to shopt for consistency with the proper builtin. Also added documentation and README entry.
Thanks again!
See go-task/task#929 for details
|
I love thanks!!! |
@andreynering Thanks for tidying this up. I forgot to add the docs/schema etc. when marking as ready. I've opened #984 to track the improvements as discussed. |
| builtins. This can be added at global, task or command level. | ||
|
|
||
| ```yaml | ||
| version: '2' |
There was a problem hiding this comment.
There was a problem hiding this comment.
@marco-m-pix4d good spot! yes, this should be version: '3'. I'll get this updated.
Summary
This PR adds 2 new directives:
set-[]string- Equivalent of the POSIXsetbuiltinshopts-[]string- Equivalent of the BashshoptbuiltinBoth of these directives are available at global, task and command level. See example Taskfile below:
Considerations
mvdan/shallows us to set POSIX shell options viainterp.Params(). However, from what I can tell, there is no equivalent for Bash options. This means that this PR is currently prepending theshoptbuiltin before each command whereshoptsare specified. I'm not a huge fan of this and have been looking at contributing a change tomvdan/shto add a newinterp.Shopts()function or similar. I'd love some input from @mvdan on whether this is a good approach.map[string]boolso that you could enable/disable opts at different levels. However, I came to the conclusion that this level of complexity wasn't really necessary as all the currently supported shell options are disabled by default anyway. Implementing as a[]stringstill allows us to enable at each level and I think this covers most use cases. It also keeps the Taskfile looking clean as we can inline the list with[].setandshoptsare definitely up for debate. I wasn't sure about the plurality for example...Currently supported shell options
POSIX (
set)If you have
goshinstalled, you can find these by runninggosh -c "set -o".allexportaerrexitenoexecnnoglobfnounsetuxtracexpipefailBash (
shopt)If you have
goshinstalled, you can find these by runninggosh -c "shopt".expand_aliasesglobstarnullglobDon't have
gosh?If you don't have
goshinstalled, you can use the following Taskfile to get these lists: